Codex automated review

One blocking schema-evolution regression found. Focused frontend tests could not run because the worktree lacks cgo/libmo.dylib.

P1 - Allow target-only historical type changes or reject them consistently (pkg/frontend/data_branch_hashdiff.go:2625)

checkSchemaCompatibility accepts a target-only column regardless of its type, so t0(a,b) and branch t1(a,b,c varchar) are compatible. However, after t1 first adds c int and then modifies it to varchar, every DIFF/MERGE walks the old physical generation and this mapping rejects old c int against endpoint c varchar before even opening CollectChanges. The column is target-only and is deliberately omitted from MERGE apply (commonVisibleIdxes), so its historical representation cannot affect t0; nevertheless all operations fail, including when there were no writes to c. Skip/project such non-common historical columns as NULL, or make the endpoint compatibility check reject this evolution up front. Add an end-to-end regression covering add-target-only-column -> modify its type -> DIFF/MERGE.

Reviewed updated head c515a14. The target-only historical type-drift blocker is closed: incompatible non-common columns are projected as NULL and hydrated from the endpoint, while common-column and primary-key drift remain rejected. DIFF and MERGE regressions cover both unchanged and updated rows. No remaining blockers found.

Codex automated review

One blocking timestamp-source race remains despite the new current-source lock.

P1 - Serialize timestamp-based data branch creation with ALTER (pkg/frontend/clone.go:461)

The new shared source lock is skipped whenever snapshot.TS is set. That includes {MO_TS=...} / timestamp sources (ResolveTsHint constructs a Snapshot with TS), not just named snapshots. Thus a timestamp-based CREATE DATA BRANCH can clone old generation A while a concurrent COPY ALTER acquires the exclusive catalog lock, sees neither branch_metadata nor a kind='user' snapshot/PITR, and omits the A→C lineage edge. Once the create commits B→A, B and current C are disconnected. DIFF/MERGE then takes the no-LCA path, whose endpoint resolver matches only name/type; after a same-name drop/re-add, it can accept and merge B's old column values into C's unrelated replacement column. Acquire the same shared lock for timestamp sources (or otherwise atomically publish the lineage before ALTER can probe), and add a concurrent {MO_TS=...} CREATE/ALTER regression test.

Codex automated review

Two blocking correctness gaps remain on c04f9e3. Prior review findings were not repeated where the current code disproves or closes them. Focused tests could not run because this environment has no Go executable.

P1 - Revalidate timestamp sources when ALTER acquires the publication lock first (pkg/frontend/clone.go:478)

The timestamp is resolved before this shared lock is acquired, and the code continues unconditionally after waiting. If COPY ALTER acquires the exclusive lock first, it can replace A with C and commit without seeing the unpublished branch; the clone then resumes and restores its already-selected historical A, publishing only B→A. No A→C lineage exists, so the successful branch later fails DIFF/MERGE as missing legacy ALTER lineage (or reaches the unrelated/no-LCA path). The new concurrency test explicitly exercises ALTER-first ordering but only asserts that the clone resumes; it does not verify generation identity or lineage closure. After acquiring the lock, reject/retry when the selected timestamp generation is no longer connected to the current source, or publish through an atomic protocol that closes A→C.

P1 - Detect same-statement DROP/ADD column redefinitions (pkg/frontend/data_branch.go:1889)

Redefinition detection only notices that a name disappears in an intermediate physical generation. MatrixOne supports multi-clause COPY ALTER, so ALTER TABLE t1 DROP COLUMN b, ADD COLUMN b INT DEFAULT 0 creates one old→new edge where both endpoint schemas contain b; this name lookup therefore treats the unrelated columns as continuous. If b was updated before the ALTER, historical mapping also maps old b into new b, allowing DIFF/MERGE to replay the stale value. Existing regressions use two separate ALTER statements and therefore do not cover this case. Preserve operation-level identity across a COPY ALTER or otherwise reject a same-name replacement even when no intermediate schema exists.

Requesting changes for two P1 correctness gaps in schema-evolution lineage. The current CI and unit/race tests pass, but the concurrency test only checks wait/resume and the DROP/ADD test uses two separate ALTER generations, so neither failure below is covered.

Requesting changes on the latest head. The same-statement DROP+ADD replacement is now rejected for data-branch lineages, which closes the previous identity-reuse issue. However, the timestamp-race fix introduces one ordinary CLONE/restore compatibility regression and still leaves revalidation non-atomic with lineage GC. Both can affect correctness, so this is not safe to approve yet.

[P1] Track same-statement column replacement by stable identity, not raw action names

The new guard can still be bypassed by a multi-clause COPY ALTER such as:

ALTER TABLE t
  RENAME COLUMN b TO tmp,
  DROP COLUMN tmp,
  ADD COLUMN b INT DEFAULT 0;

buildAlterTableCopy resolves the recorded drop against the original tableDef by the action name. After the rename, tmp exists only in the evolving CopyTableDef, so DROP tmp produces no DropColumn action. The detector then sees only ADD b and does not reject the replacement. The lineage walk still matches the old and new b by name, so DIFF/MERGE can project an old branch value into the semantically new column.

Please track the dropped/added column through the evolving schema using stable identity (including later renames), or validate the final old-to-new identity mapping. Add DIFF/MERGE coverage for rename -> drop -> add and drop -> add -> rename variants.

Codex automated review

The latest changes close the previously reported direct DROP+ADD case and post-lock generation re-resolution, but four blocking compatibility/correctness gaps remain.

P1 - Scope timestamp lineage validation to DATA BRANCH clones (pkg/frontend/clone.go:573)

This block is keyed only on the timestamp snapshot, whereas the preceding lock correctly checks dataBranchCloneLockCtxKey. It therefore also runs for ordinary CREATE TABLE ... CLONE ... {MO_TS=...} and restore-generated clones. After an ordinary copy ALTER A→C with no historical owner, no A→C branch edge is retained; a still-valid pre-ALTER timestamp resolves A while current resolves C, so normal clone/restore now fails with “generation is not connected”. Gate this revalidation on explicit data-branch mode.

P1 - Hold the validated lineage through branch publication (pkg/frontend/clone.go:169)

constructBranchDAG performs a plain SELECT and may use an independent internal executor, so the validated rows are not locked by the transaction that later publishes branch metadata. A clone can read C→A and accept A, concurrent lineage GC can delete C→A and its snapshot, and then the clone can insert B→A and its distinct snapshot without a write conflict. The committed branch B is disconnected from current C again. Lock the required lineage rows in the publication transaction or atomically register the new owner.

P1 - Detect rename-then-name-reuse replacements in one ALTER (pkg/sql/compile/alter.go:685)

The new guard intersects only explicit DropColumn and AddColumn actions. ALTER TABLE t RENAME COLUMN b TO old_b, ADD COLUMN b INT DEFAULT 0 records ADD b but no drop, so it passes with active lineage. Across the resulting lineage-only edge, dataBranchColumnReachesLCA matches the new b to ancestor b by name while treating old_b as target-only; DIFF/MERGE can therefore compare or apply the replacement column and historical projection can move old b values into it. Track rename/change identity transitions as well as drops.

P1 - Allow OUTPUT AS to materialize target-only columns (pkg/frontend/data_branch_output.go:420)

Schema reconciliation now permits target-only columns, and an unqualified DIFF includes them in its target-layout output. OUTPUT AS defaults to all target-visible columns, but createSQL maps every projected target column back to the base by ColId/Seqnum. A newly added target column has no base identity, so the otherwise valid schema-evolved DIFF fails before creating the output table. Build target-only definitions from the target schema while retaining the mapped base definition for common/renamed columns.

Codex automated review

Two concurrency races can leave data-branch lineage disconnected or delete history needed by a newly created PITR.

P1 - Revalidate timestamp sources after database-clone locks (pkg/frontend/clone.go:777)

Database cloning resolves its source before acquiring the sorted shared locks at lines 721-725. Nested table clones then set dataBranchCloneLockCtxKey to false here; that disables both duplicate locking and shouldRevalidateTimestampDataBranchCloneSource. If COPY ALTER publishes a new generation between source collection and lock acquisition, the nested clone still restores the old generation at the MO_TS and updateBranchMetaTable records that old ID as the parent. When no prior branch or historical owner exists, ALTER creates no connecting lineage edge, so the new branch is published against a disconnected generation and later DIFF/MERGE cannot safely establish its LCA. Revalidate the complete database source set once after all locks are held, while retaining the no-reacquire behavior for nested clones.

P1 - Synchronize PITR creation with lineage compaction (pkg/sql/compile/alter.go:367)

The compaction source query reads active PITR rows without locking them, while compaction only locks mo_branch_metadata. CREATE/activate PITR does not acquire that metadata lock, so a PITR can become active after this read but before the subsequent snapshot and metadata deletes. If its retention lower bound covers an ALTER generation, the stale plan still deletes that generation and its branch snapshot, making the newly created PITR unable to restore it. The source set must be synchronized with compaction (or revalidated while holding the deletion lock) before deleting lineage.

Codex automated review

The latest database timestamp-source and PITR-publication fixes close the two re-review findings, but named-snapshot branch creation still has a blocking lineage-publication race.

P1 - Make named-snapshot lineage ownership atomic with snapshot deletion (pkg/frontend/clone.go:620)

Revalidation and the mo_branch_metadata FOR UPDATE lock only run for timestamp hints; named snapshots (ExtraInfo != nil) skip both paths. Consider snapshot S created on physical generation A, followed by a snapshot-covered COPY ALTER A→C that records lineage C→A. A DATA BRANCH clone can resolve S, then concurrent DROP SNAPSHOT can delete S and run compactHistoricalAlterLineageWithBH, removing C→A before the clone publishes its independent B→A metadata/snapshot rows. The clone uses the already-resolved timestamp, so it can still succeed, but B is now disconnected from current C and later DIFF/MERGE cannot establish the correct LCA. This affects table and database clones. Lock/revalidate the named-snapshot lineage through branch publication, or atomically transfer ownership from the snapshot to the new branch.

Codex automated review

Reviewed 5dda117 against origin/main and the full existing review discussion. The named-snapshot locking and revalidation fix closes the prior blocker; no new actionable regressions found.